Skip to content

refactor(commands): move commands-side rendering out of src/daemon and retire the doctor progress flag - #2349

Merged
thymikee merged 4 commits into
mainfrom
claude/2337-commands-rendering-out-of-daemon
Sep 6, 2026
Merged

refactor(commands): move commands-side rendering out of src/daemon and retire the doctor progress flag#2349
thymikee merged 4 commits into
mainfrom
claude/2337-commands-rendering-out-of-daemon

Conversation

@thymikee

@thymikee thymikee commented Sep 6, 2026

Copy link
Copy Markdown
Member

Summary

Closes #2337.

src/commands and src/mcp reached into src/daemon for rendering they own — 11 import() sites, 5 file edges. Each moves to its owner:

  • daemon/result-serialization.tscommands/output/result-serialization.ts; the two deployment target resolvers its daemon handlers needed → core/deploy-result-target.ts.
  • daemon/handlers/doctor-output.ts and handlers/status-markers.tssrc/core/, below the daemon handlers, the replay-test reporter and the doctor formatter.
  • daemon/client/doctor-progress.ts (a process-global flag) is deleted, not deprecated. The CLI installs createStderrCommandProgressSink as the request's RequestProgressSink and hands its CommandProgressState to the output formatters; the daemon client only forwards events to the caller's sink.

The MCP edge becomes static. The src/commands edges stay function-scoped: static would take src/cli.ts from 380 to 384 eager modules, which the no-growth budget rejects. No budget raised, no compatibility re-export, no gate or baseline edit needed.

Validation

Tested at e4bc1ea: pnpm install --frozen-lockfile && pnpm build, pnpm typecheck, pnpm lint, pnpm check:affected --run — all runnable checks passed (270 files / 2025 tests, plus daemon-wire-compat).

Graph query over pnpm depgraph output (non-type edges, src/commands|src/mcp → src/daemon): 5 at 27a97ee, 0 at head — query, both outputs and the rename-only stat in the first comment.

pnpm check:layering: OK, 0 R5 back-edges, R2 clean. Eager-closure budgets: 442 pass; all 225 entries unchanged, src/cli.ts 380 → 380.

Doctor CLI output is byte-identical in all three modes (progress streamed, none, custom sink); captures diffed empty. The new CLI-level test was verified failing without the fix.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ


Generated by Claude Code

…their callers

`formatDoctorCheckSummaryLine`, `formatDoctorCheckDetailLines`, and
`formatCliStatusMarker` are shared by the daemon doctor handlers, the CLI
replay-test reporter, and the doctor CLI output formatter. Owning them in
`src/daemon/handlers/` forced `src/commands` to reach up into `src/daemon`
through a dynamic import; `src/core` sits below every caller.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ
The `serialize*` helpers render command results for the CLI and MCP surfaces,
not for the daemon; only the two deployment target resolvers were shared with
daemon handlers. The serializers move to `src/commands/output/` beside the
formatters that call them, and the resolvers move to `src/core` below both
owners, so `src/commands` and `src/mcp` no longer reach into `src/daemon` for
them. The MCP edge becomes a static import; the `src/commands` edges stay
function-scoped because `src/cli.ts` eagerly evaluates both output modules.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ
…ng the doctor progress flag

Streamed `command` progress was rendered to stderr by the daemon client, which
recorded "doctor progress was rendered" in a process-global flag that
`doctorCliOutput` then consumed to suppress its final summary. The rendering was
commands-side, so it moves there: the CLI installs `createStderrCommandProgressSink`
as the request's `RequestProgressSink` for every non-`test`, non-`--json` command,
and hands its `CommandProgressState` to the output formatters with the result.
The transport now only forwards events to the sink the caller installed.

Behavior is unchanged in the three modes the tests pin: progress streamed to
stderr suppresses doctor's check lines, no progress prints them, and a caller
that installs its own sink (SDK, MCP) writes nothing to this process's stderr
and is not suppressed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ

thymikee commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

Completion evidence

Zero edges from src/commands / src/mcp into src/daemon

Query over pnpm depgraph output (node ids drop the src/ prefix; kind 1 is type-only, which the condition excludes):

const g = JSON.parse(fs.readFileSync(process.argv[2], 'utf8'));
const KIND = ['value', 'type', 'dynamic'];
const rows = g.edges
  .map(([from, to, kind]) => ({ from: g.nodes[from].id, to: g.nodes[to].id, kind: KIND[kind] }))
  .filter((e) =>
    (e.from.startsWith('commands/') || e.from.startsWith('mcp/')) &&
    e.to.startsWith('daemon/') && e.kind !== 'type');
27a97ee: value+dynamic edges src/commands|src/mcp -> src/daemon: 5
  dynamic  src/commands/capture/output.ts -> src/daemon/result-serialization.ts
  dynamic  src/commands/management/output.ts -> src/daemon/client/doctor-progress.ts
  dynamic  src/commands/management/output.ts -> src/daemon/handlers/doctor-output.ts
  dynamic  src/commands/management/output.ts -> src/daemon/result-serialization.ts
  dynamic  src/mcp/tool-result.ts -> src/daemon/result-serialization.ts

e4bc1ea: value+dynamic edges src/commands|src/mcp -> src/daemon: 0

(5 file edges = the 11 import() sites the issue inventories; the graph dedupes repeats between the same pair.)

Eager-closure budgets

scripts/__tests__/eager-closure-budgets.test.ts: 442 passed. Measured over all 225 entries the gate covers, before vs. after: no entry changed; src/cli.ts stays at 380 modules (daemon/client/doctor-progress.ts leaves the closure, commands/command-progress.ts enters it).

Measured delta for the two src/commands edges kept lazy: making them static takes src/cli.ts 380 → 384 (commands/output/result-serialization.ts, core/doctor-output.ts, core/status-markers.ts, core/deploy-result-target.ts), which the no-growth rule rejects — so the lazy edges stay and no budget was raised. The src/mcp/tool-result.ts edge is under no measured entry's closure, so it became static.

Doctor CLI output, three modes

Both sides drive readDaemonSocketProgressResponse and doctorCliOutput exactly as their own CLI wires them (main: no sink, the client renders and sets the global flag; here: the CLI's createStderrCommandProgressSink plus its state). Captured stderr + text + data, diff empty:

--- mode 1 progress streamed to stderr ---
stderr:
✓ agent-device: agent-device 0.17.9 using /tmp/agent-device
! device: No booted device.
text:
Doctor: pass
No blockers found.
--- mode 2 no progress ---
stderr:
text:
Doctor: pass
✓ agent-device: agent-device 0.17.9 using /tmp/agent-device
! device: No booted device.
  hint: Boot a simulator.
--- mode 3 custom sink ---
stderr:
text:
Doctor: pass
✓ agent-device: agent-device 0.17.9 using /tmp/agent-device
! device: No booted device.
  hint: Boot a simulator.
custom sink events:[{"type":"command",…},{"type":"command",…}]

Mode 1 and 2 also get an end-to-end CLI pin (src/__tests__/cli-network.test.ts), verified failing with the suppression branch disabled; mode 3 is pinned at both seams (command-progress.test.ts, management/output.test.ts).

Rename-only proof

git diff -M90% --stat origin/main...HEAD — 30 files, 407 insertions, 231 deletions:

 src/{daemon/handlers => core}/doctor-output.ts     |   0
 src/{daemon/handlers => core}/status-markers.ts    |   0
 src/{daemon => commands/output}/result-serialization.ts      |  21 +---
 src/{daemon/__tests__ => commands/output}/snapshot-serialization.test.ts |   2 +-

The two pure moves are content-identical. result-serialization.ts loses only the two resolver functions extracted to core/deploy-result-target.ts; its test file splits the same way (the 132-line delete / 102-line add pair), so both halves carry their existing assertions unchanged.


Generated by Claude Code

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.49 MB 4.49 MB -160 B
Package (unpacked) 4.49 MB 4.49 MB -160 B
Package (download) 1.33 MB 1.33 MB -7 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 25.4 ms 25.0 ms -0.3 ms
CLI --help 73.4 ms 73.4 ms -0.0 ms

The end-to-end doctor progress cases landed in `cli-network.test.ts`, which is
already past the 1,000-line tripwire, so the test-file-size ratchet rejected the
growth. They are doctor-output cases rather than network ones: they move to
`cli-doctor-progress.test.ts`, and `cli-network.test.ts` returns to its
merge-base size.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ
@thymikee

thymikee commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

Clean review at 29f3527. The shared helpers and serializers have clear owners, and the doctor progress state is local to each CLI run. Streamed output avoids duplicates; custom sinks still get the full result. The tests cover both streamed and unstreamed CLI output, and the old global flag is gone.

Ready for human review. Remaining CI is still running, so this is not yet a merge-ready claim.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 6, 2026
@thymikee
thymikee merged commit 6768a04 into main Sep 6, 2026
17 of 18 checks passed
@thymikee
thymikee deleted the claude/2337-commands-rendering-out-of-daemon branch September 6, 2026 09:30
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-06 09:31 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(commands): move commands-side rendering out of src/daemon and retire the doctor progress flag

2 participants